Refactor: extract a shared listSessions pagination helper into base_session_service - #616

Please ensure you have read the contribution guide before creating a pull request.

Link to Issue or Description of Change

1. Link to an existing issue (if applicable):
   N/A — no public issue tracks this refactor.
2. Or, if no issue exists, describe the change:
   Problem: The pagination arithmetic behind listSessions is copy-pasted across every session backend. InMemorySessionService, VertexAiSessionService and DatabaseSessionService each independently compute effectiveOffset, effectivePage and totalPages, and each re-implements the limit === 0 and limit === undefined special cases. The three copies have already drifted in shape — the in-memory copy carries a whole duplicate branch for the "app or user not found" case, Vertex has no such branch, and the database copy spells the same arithmetic a third way. Every new backend has to re-derive it, and every fix has to be applied three times.

Solution: Collapse the arithmetic into two exported helpers in core/src/sessions/base_session_service.ts, alongside the existing mergeStates / trimTempState:

• resolvePagination(request, totalItems) — pure arithmetic. Turns a ListSessionsRequest plus a known total into the slice offset and the response metadata. For backends that paginate in their storage layer, where the offset and the request's own limit map onto OFFSET/LIMIT.
• paginateSessions(sessions, request) — sorts, slices and wraps, built on resolvePagination. For backends that hold the whole result set in memory.

Then converge the three landed backends onto them. Net effect on core/src/sessions/: −188 lines of duplicated arithmetic, +137 lines of single implementation, for a net −51.

Design notes:

• The database backend keeps its query strategy. It adopts resolvePagination for the numbers only, and still pushes limit/offset down into em.find. Converging it onto paginateSessions would read as tidier but would mean em.find-ing the entire session table into Node memory on every listSessions call — an unbounded-memory regression on the one backend actually backed by a database. The arithmetic is what is shared; the query strategy is a storage-layer concern.
• The in-memory "app/user not found" branch is deleted, not ported. Running the general algorithm over an empty array produces identical output for every input: with no limit it yields {page: 1, limit: 0, totalItems: 0, totalPages: 0} because limit is reported as totalItems; with a limit, totalPages is limit === 0 ? 0 : Math.ceil(0 / limit), which is 0 either way, and [].slice(x, y) is []. Three new tests pin the deleted branch's outputs directly (empty input with no params, with a limit, and with a limit + page).
• paginateSessions does not mutate its input. It sorts a shallow copy, and only when order is set. The current call sites all pass freshly built local arrays so this is not observable today, but a shared helper that silently reorders a caller's array is a trap. Pinned by a test.
• This is a pure refactor — no observable behaviour changes, including the quirks: limit === undefined reports limit: totalItems; limit: 0 returns an empty page but a truthful totalItems; page beats offset; totalPages is 0 (not 1) for an empty result set; negative and out-of-range values are still passed through unvalidated (page: 0 still yields a negative slice start). Adding validation would be a behaviour change and belongs in its own PR.
• Not added to core/src/index.ts. These are internal cross-module helpers consumed by sibling files via relative imports, exactly like mergeStates. Public signatures of the three listSessions methods are byte-identical before and after.

Convergence acceptance check — core/src/sessions/ now holds exactly one copy of the arithmetic:

$ grep -ln "Math.floor(effectiveOffset / limit)\|Math.floor(offset / limit)" core/src/sessions/*.ts
core/src/sessions/base_session_service.ts

Collision check (required before starting; recorded here either way). Searched all 514 open PRs on the fork:

gh pr list --repo AmaadMartin/adk-js --state open --limit 1000 --json number,title,headRefName

No open PR extracts or shares the pagination arithmetic — zero hits for "paginat" across every open title and branch name. Three open PRs touch the same files with a different concern and were checked by diff: #376 (makes ListSessionsRequest.userId optional), #477 (aligns the listSessions state contract, stacked on #376) and #609 (extracts a shared extractStateDelta helper). None of them touches the pagination arithmetic, so this branches from main rather than stacking. #376 does independently delete the same in-memory "app/user not found" branch, so whichever of the two lands second needs a one-hunk rebase in in_memory_session_service.ts.

Testing Plan

Please describe the tests that you ran to verify your changes. This is required for all PRs that are not small documentation or typo fixes.

Unit Tests:
[x] I have added or updated unit tests for my change.
[x] All unit tests pass locally.

New file core/test/sessions/base_session_service_test.ts (24 cases) covers both helpers directly: the documented no-params contract, empty input with and without a limit and page (the deleted in-memory branch), limit only, offset only, limit + offset, page + limit, page beating offset, limit: 0, an offset beyond the total, both sort directions, the id.localeCompare tie-break in both directions, order-omitted passthrough, and non-mutation of the input array. resolvePagination is additionally exercised on its own — that is the surface the database backend uses, and it must be covered without going through paginateSessions.

One case was added to core/test/sessions/database_session_service_test.ts: offset without limit. That query branch had no test at all, so nothing pinned the metadata it now gets from resolvePagination.

No existing test was modified. The ~10 pagination cases in in_memory_session_service_test.ts, the listSessions pagination and sorting block in database_session_service_test.ts and the Vertex cases in vertex_ai_session_service_test.ts are the regression net that proves the refactor is behaviour-preserving; all 158 tests in core/test/sessions/ pass unmodified.

Coverage. 100% of the new code in base_session_service.ts (statements, branches, functions) and of all three rewritten listSessions bodies, measured with @vitest/coverage-v8 over core/test/sessions/. The only uncovered lines remaining in base_session_service.ts are pre-existing (getOrCreateSession, appendEvent, updateSessionState), untouched by this change.

Proof the tests can fail. Coverage is a floor, not proof, so each mutation below was applied to the helper one at a time and sequentially, the suite re-run, and the mutation reverted. After the last revert, git diff HEAD was empty — no mutation survived.

Manual End-to-End (E2E) Tests:
No integration test is added: this refactor performs no I/O and crosses no process boundary. The DatabaseSessionService suite already runs end to end against a real in-memory SQLite database (SqliteDriver, dbName: ':memory:'), so it is the integration-level proof that resolvePagination's numbers still produce correct SQL LIMIT/OFFSET.

To reproduce locally:

npm install
npm run build                                          # ok
npx vitest run --project unit:core core/test/sessions/ # 7 files, 158 tests passed
npm run lint                                           # clean
npm run format:check                                   # all files use Prettier code style
npm run docs:check                                     # clean (typedoc, treatWarningsAsErrors)
grep -ln "Math.floor(effectiveOffset / limit)\|Math.floor(offset / limit)" core/src/sessions/*.ts
# -> core/src/sessions/base_session_service.ts   (exactly one file)

npm run ts:check reports 281 errors, but that is the pre-existing state of the test tree on main (unresolvable @google/adk/... subpath imports and dist vs src type identity — the subject of separate open PRs). The count is 281 before and 281 after this change, and none of them are in the files this PR touches.

Checklist

[x] I have read the CONTRIBUTING.md document.
[x] I have performed a self-review of my own code.
[x] I have commented my code, particularly in hard-to-understand areas.
[x] I have added tests that prove my fix is effective or that my feature works.
[x] New and existing unit tests pass locally with my changes.

Review round 1 — complexity

A simplicity audit raised four non-blocking findings; all four are implemented in commit Refactor: shrink the pagination helpers per review:

1. ResolvedPagination.take deleted. It was always exactly request.limit, and no storage backend read it — DatabaseSessionService passes its own destructured limit to em.find. Every caller already holds the request, so paginateSessions now reads limit from there. This is a deliberate departure from the approved spec's data model, which listed take?: number: the field had no reader outside the module that produced it. ResolvedPagination is now {offset, meta}.
2. The take === undefined ternary folded into slice's optional end. slice(start, undefined) already means "to the end"; limit: 0 still yields [] via slice(offset, offset).
3. The two sign-mirrored comparators collapsed into one scaled by a direction factor. -0 is falsy, so the id.localeCompare tie-break still applies to equal timestamps in both directions — pinned by mutations 5 and 8.
4. The two limit-less database branches merged, since they now differ only in how totalItems is obtained. The no-COUNT fast path is preserved and is pinned by mutation 10.

Four resolvePagination assertions in the new test file dropped their take expectation, since the field no longer exists. No pre-existing test was touched: git diff main -- core/test/ contains zero deleted lines.

CI

All test jobs pass on the current head: run-tests (ubuntu-latest, macos-latest, windows-latest) plus the aggregate run-tests job.

Two known intermittent timeouts on the non-Linux runners needed a re-run and are unrelated to this change — it touches only core/src/sessions/** and core/test/sessions/**, and each failing run was 1 failure out of ~2700 tests:

• tests/integration/app_loader/app_loader_test.ts > should discover apps vs agents across directories and standalone files, Test timed out in 40000ms (macos-latest).
• core/test/code_executors/unsafe_local_code_executor_test.ts > should execute shell code and return stdout, Test timed out in 5000ms (windows-latest).

ubuntu-latest and the aggregate run-tests job passed on every attempt.